refactor(account-settings): refactor formValidation, updateAccountSettings & dataProcessing #242
+156
−185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…essing methods Current code uses `isEdit`, but then exposes it as `!isEdit`, this is ultimately confusing. Change the flag name to pristine, and change the method to either return the payload, if we need to save it, or undefined if not modified. B2B-2158
…sswordModifiedFields These two methods are used at different stages of the process and for different reasons. Separating them allows us to only execute them when they are needed and more importantly _when_ they are needed. eg. the getAccountSettingFiles is not required for bc users. Remove the configuration for the `xs` prop, the only use of it is `12`. Translate the passwordFields directly upon receiving them, as opposed to doing it in the component. Hard code the list of fields directly in the method, we had a method for indirection that wasn't needed. Bonus: flip all the `!isBcUser` ternaries B2B-2158
…cations The current logic conflates form validation with form error display/translations. We should probably include some form library, but for now we'll just separate the methods doing the validation from the methods displaying the errors/alerts. B2B-2158
Separate payload preparation from the update process. Purify and simplify handleGetUserExtraFields. B2B-2158
icatalina
force-pushed
the
icatalina/B2B-2150/account-settings/1
branch
from
af00a9c to
a933208
Compare
| accountB2BFormFields, | ||
| passwordModified, | ||
| }; | ||
| export const getPasswordModifiedFields = (b3Lang: LangFormatFunction): GetFilterMoreListProps[] => { |
There was a problem hiding this comment.
any benefit in turning these into a hooks and getting b3Lang internally rather than by argument?
| channelId, | ||
| }; | ||
|
|
||
| const { isValid }: { isValid: boolean } = isBCUser |
There was a problem hiding this comment.
is the type definition { isValid: boolean } here because the async functions are giving back any?
If so, I'd be tempted to move it closer to the problem by adding it as a return type definition of their signatures
(rather than make this function make up for their shortcomings)
| const emailFlag = emailValidation(data); | ||
|
|
||
| if (!emailFlag) { |
There was a problem hiding this comment.
these flag names aren't very descriptive, what do you think to turning them into something like
if (isValidEmail(data.email)) {
// etcin another commit/PR/ticket?
| @@ -132,6 +130,7 @@ function AccountSetting() { | |||
|
|
|||
| const { [key]: accountSettings } = await fn(params); | |||
|
|
|||
| const accountFormAllFields = await getB2BAccountFormFields(isBCUser ? 1 : 2); | |||
There was a problem hiding this comment.
"how many fields do you want? One, two..?" 😆
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Jira: B2B-2158
What/Why?
refactor(account-settings): reorganise account settings update
Separate payload preparation from the update process.
Purify and simplify handleGetUserExtraFields.
refactor(account-settings): separate formValidation from error notifications
The current logic conflates form validation with form error
display/translations.
We should probably include some form library, but for now we'll just
separate the methods doing the validation from the methods displaying
the errors/alerts.
refactor(account-settings): separate getAccountSettingsFields & getPasswordModifiedFields
These two methods are used at different stages of the process and for
different reasons. Separating them allows us to only execute them when
they are needed and more importantly when they are needed.
eg. the
getAccountSettingFilesis not required for bc users.Remove the configuration for the
xsprop, the only use of it is12.Translate the passwordFields directly upon receiving them, as opposed to
doing it in the component.
Hard code the list of fields directly in the method, we had a method for
indirection that wasn't needed.
Bonus: flip all the
!isBcUserternariesrefactor(account-settings): rename isEdit to pristine in the dataProcessing methods
Current code uses
isEdit, but then exposes it as!isEdit, this isultimately confusing. Change the flag name to
pristine, and change themethod to either return the payload, if we need to save it, or undefined
if not modified.
Rollout/Rollback
Revert
Testing
Manual